Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript declarations #98

Closed

Conversation

pineapplemachine
Copy link
Contributor

@pineapplemachine pineapplemachine commented Jan 11, 2019

This PR includes several documentation fixes that also fix TypeScript declaration issues.

I used this script to transform the grunt output into a syntactically valid TypeScript declaration file. I've tested to the extent that my project using a tiny slice of the API doesn't immediately die. Any further errors are probably mistakes in the documentation. For example, I only ran into TextureOptions.color incorrectly being marked as a mandatory attribute (when in fact it is optional) because I happen to be using TextureOptions in my small project that I used to help with testing, and I got a compilation error complaining that I hadn't included a color attribute.

I'm not familiar with your build process @greggman so I'm gonna go ahead and request that you take this across the finish line, incorporating this script (...probably rewritten in JS. sorry, I'm faster at text processing in Python) as part of the TS declarations build process. You might also consider making some of these changes in the documentation directly, for example replacing "?" to document functions that accept an arbitrary type with "any"; and/or replacing "enum" with the perhaps more conventional "GLenum". (Right now, the script must change these things in the declarations file.)

import re

# Read from file built by grunt
with open("twgl.js.d.ts_wip", "r") as input_file:
    content = input_file.read()

# Remove docstrings (declarations do not by convention include these)
content = re.sub(r'(?s)/\*\*.*?\*/\s*', "", content)

# Docs use "?" to represent an arbitrary type; TS uses "any"
content = re.sub(r'\]: \?', "]: any", content)

# Docs use "constructor"; TS expects something more like "Function"
content = re.sub(r': constructor', ": Function", content)

# Docs use "ArrayBufferViewType" to describe a TypedArray constructor
content = re.sub(r'\bArrayBufferViewType\b', "Function", content)

# What docs call "TypedArray", lib.d.ts calls "ArrayBufferView"
content = re.sub(r'\bTypedArray\b', "ArrayBufferView", content)

# What docs call an "augmentedTypedArray" is technically an "ArrayBufferView"
# albeit with a patched-in "push" method.
content = re.sub(r'\baugmentedTypedArray\b', "ArrayBufferView", content)

# Docs use "enum"; TS expects "GLenum"
# https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/Types
content = re.sub(r': enum', ": GLenum", content)

# Remove every instance of "module:twgl" and "module:twgl/whatever"
content = re.sub(r'module:twgl(/\w+)?\.', "", content)

# Remove all "declare module twgl" and "declare module twgl/whatever"
# It should be enough simply for the declarations to reside in a "twgl.js.d.ts" file
content = re.sub(r'declare module twgl {\s*', "", content)
content = re.sub(r'}\s*$', "", content)
content = re.sub(r'(?s)}[^{}]*?declare module twgl/\w+ {', "", content)

# De-indent cause all that left-over whitespace is driving me bonkers
content = re.sub(r'(?m)^ {4}', "", content)

# Replace "function", "type" declarations with "export function", "export type"
content = re.sub(r'(?m)^(function|type) ', "export \\1 ", content)

# Fixup dynamically generated glEnumToString function signature
glEnumToString = """
export function glEnumToString(gl: WebGLRenderingContext, value: number): string;
"""
content = re.sub(r'var glEnumToString: any;', glEnumToString, content)

# Add missing type (describes canvas.getContext input attributes)
# https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/getContext
content = """
export interface WebGLContextCreationAttirbutes {
    alpha?: boolean;
    antialias?: boolean;
    depth?: boolean;
    failIfMajorPerformanceCaveat?: boolean;
    powerPreference?: string;
    premultipliedAlpha?: boolean;
    preserveDrawingBuffer?: boolean;
    stencil?: boolean;
}
""" + content

# Write to destination test file
with open("twgl.js.d.ts", "w") as output_file:
    output_file.write(content)

greggman and others added 5 commits January 12, 2019 01:55
this uses the tsd-jsdoc npm package
https://www.npmjs.com/package/tsd-jsdoc

it gets a few errors and no idea how close the types
are. Maybe someone more familar with typescript can take a look?

checked in the generated types here

You can run it with

    npm install
    ./node_modules/.bin/grunt jsdoc:ts
/** comment */ was being confused as a docstring and resulting in TS declaration pollution
@pineapplemachine
Copy link
Contributor Author

pineapplemachine commented Jan 11, 2019

See also #50 #89 #97

@pineapplemachine pineapplemachine changed the title Sophie/gen ts TypeScript declarations Jan 11, 2019
@NikitaIT
Copy link

NikitaIT commented Jan 12, 2019

it is worth saying that although this method is good at the initial stage, it does not give the desired severity of the types.

-> @return {Object.<string, WebGLBuffer>} The created sphere buffers

<string, here you need to specify the properties explicitly

@return {{
    position: WebGLBuffer,
    normal: WebGLBuffer,
    texcoord: WebGLBuffer,
    indices: WebGLBuffer,
  }} The created sphere buffers

for

Example:
     *
     *         const bufferInfo1 = twgl.createBufferInfoFromArrays(gl, {
     *           position: [1, 2, 3, ... ],
     *         });
     *         const bufferInfo2 = twgl.createBufferInfoFromArrays(gl, {
     *           position: bufferInfo1.attribs.position,  // use the same buffer from bufferInfo1
     *         });

This case should be taken into account in the typing, which I showed in my example
I'm not a big expert on how to do this through js-doc, but in ts..

@NikitaIT
Copy link

I think a good solution would be to generate files for each file in src and correct them, and then make into Assembly by the ts compiler. Although this will require a lot of effort, it will ease the work with d.ts in the future.

@pineapplemachine
Copy link
Contributor Author

pineapplemachine commented Jan 12, 2019 via email

@greggman
Copy link
Owner

greggman commented Jan 13, 2019

I incorporated this PR into #97

Your python translated to node/js is here

let content = fs.readFileSync('dist/4.x/types.d.ts', {encoding: 'utf8'});

I tested it builds. I didn't test that it runs or that I translated every regex correctly. To build

npm install
npm run build

To just build the type file

npm run buildts

@greggman
Copy link
Owner

<string, here you need to specify the properties explicitly

@return {{
    position: WebGLBuffer,
    normal: WebGLBuffer,
    texcoord: WebGLBuffer,
    indices: WebGLBuffer,
  }} The created sphere buffers

this doesn't feel right to me.

To me they should be documented as returning a map of strings to WebGLBuffer like

 Record<string, WebGLBuffer>

@pineapplemachine
Copy link
Contributor Author

Closing this PR; please see #102 instead

@NikitaIT
Copy link

NikitaIT commented Jan 15, 2019

Record<string, WebGLBuffer>

Why? this makes the code more complex
in ts you always rely on the types, and this type can not be relied on,
this at least contradicts the Occam's razor principle of good code

perhaps this description will look better, this is the option I showed in ts

Record<'position | 'normal' | 'indices' | 'texcoord', WebGLBuffer>

@pineapplemachine
Copy link
Contributor Author

pineapplemachine commented Jan 15, 2019 via email

@NikitaIT
Copy link

@pineapplemachine I'm not saying it's worth doing right now) but it's definitely worth it

@greggman
Copy link
Owner

greggman commented Jan 16, 2019

I'd like to understand the call for stronger types. Record<string, WebGLBuffer> vs

@return {{
    position: WebGLBuffer,
    normal: WebGLBuffer,
    texcoord: WebGLBuffer,
    indices: WebGLBuffer,
  }} 

I have lots of C++/C# experience so I'm not allergic to types I don't really understand a hard coded type makes sense for attribute/buffer related things.

it's perfectly valid to do this

const buffers = createSphereBuffers(...);
buffers.colors = someWebGLBuffer;

Typing createSphereBuffers as returning some specific list doesn't make sense to me but maybe I don't understand the suggestion. In C++/C# making a hard coded type with 4 explicit proprties would mean adding a 5th property is prohibited. Is this different in typescript?

All the functions that deal with buffers, attributes, and uniforms are explicitly string based maps to objects. There is no special meaning . You can make a shader

attribute float foo;
attribute vec3 bar;

and then make arrays

const bufferInfo = twgl.createBufferInfoFromArrays(gl, {
  foo: { numComponents: 1, data: [1,2,3,4] },
  bar: { numComponents: 3, data[ 1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 11, 12],
};

In general position, texcoord, normal etc are not special

Similarly you're equally free to do something like

const sphereArrays = twgl.primitives.createSphereArrays(10,20,30);
const arrays = {
     foo: { numComponents: 1, data: [1,2,3,4, ...] },
     bar: sphereArrays.position,
};

or like above

const sphereArrays = twgl.primitives.createSphereArrays(10,20,30);
sphereArrays.colors =  { numComponents: 3, data: [1,2,3,4, ...] };

I don't mind stronger types if typescript has some special way of saying "this thing is a map of strings to X and starts with these specific 4 things but youre free to add or remove more things to the map" but coming from C++/C# that can't be expressed there AFAIK so I'm assuming it also can't be expressed in typescript?

@pineapplemachine
Copy link
Contributor Author

pineapplemachine commented Jan 16, 2019

@greggman There are tools to work around that sort of situation in TypeScript, since it's just JS and dynamic typing under the hood. Here is one option that I think could work better than simply <string, WebGLBuffer> and may address your concerns:

interface ObjectWithAtLeastTheseProperties {
    a: string;
    b: string;
    c: string;
    [key: string]: string;
}

function doStuff() {
    const abc: ObjectWithAtLeastTheseProperties = {
        a: "apple",
        b: "bear",
        c: "clue",
    };
    abc.d = "door";
}

@pineapplemachine
Copy link
Contributor Author

pineapplemachine commented Jan 16, 2019

Another option is leaving it up to the dependent code to specify if they want to add any more properties to the object or not. For example, you could strictly define the input type as was first discussed and then in cases where somebody wanted to mess with the object's properties, they'd write something more like this:

const buffers = <[key: string]: WebGLBuffer> createSphereBuffers(...);
buffers.colors = someWebGLBuffer;

Or for even more freedom, something like this:

const buffers = <Object> createSphereBuffers(...);
buffers.colors = someWebGLBuffer;
buffers.whatever = "Hello, world!"

@NikitaIT
Copy link

NikitaIT commented Jan 19, 2019

1) {  a?: T, b: T } &  {  a: T } === {  a: T, b: T }
2) { [key: keyof { a?: T, b: T } ]: K} === { a?: K, b: K }
3) <S extends { a?: T, b: T }>{ [key: keyof S ]: K} 
4) M = <S>{ [key: keyof (S & { a?: T, b: T }) ]: K} 
5) H = <S>{ [key: keyof (S & { a?: T, b: T }) ]: W}
6) G = <S, L>{ [key: keyof (S & { a?: T, b: T }) ]: L}
7) fn<A>(a: G<A, []>) : G<A, WebglBuffer>
8) buffers = fn(array) // fn<A> auto derived

See code not see where in it user-defined type of

return {

return {
    position: positions,
    normal: normals,
    texcoord: texcoords,
    indices: indices,
  };

This is TRecord = Record<'position' | 'normal' | 'indices' | 'texcoord', WebGLBuffer>.

But user-defined type of

function createBuffersFromArrays(gl, arrays) {

createBuffersFromArrays<TRecord, TUserArrayNames>(gl, arrays: Record<keyof TRecord & TUserArrayNames, []>): TRecord & Record<TUserArrayNames, WebGLBuffer> {}
// TUserArrayNames is derived from the arrays

or TUserArrayNames = keyof TUserArrayByName

and user use like:

const arrays: { abs: [], position: [] } = ... ;
const buffers:  Record<'position' | 'abs', WebGLBuffer> = createSphereBuffers(...); // and this type is derived from the arguments(arrays)

in c++ or c# u write code like

enum NameEnum {
    position,
    normal,
    texcoord,
    indices,
}
class Buffer<TNameEnum = NameEnum> whare TNameEnum: Enum {
    ...
   public WebGLBuffer this[TNameEnum i] 
   { 
     get { return arrayByName[i]; }  
   }
}

and user can create self enum or extends(i know what enum not extendable, but exists many ways escape this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants